Skip to content
This repository has been archived by the owner on Feb 6, 2020. It is now read-only.

Cls error masking #67

Open
wants to merge 4 commits into
base: development
Choose a base branch
from
Open

Cls error masking #67

wants to merge 4 commits into from

Conversation

nicholasturner1
Copy link
Contributor

Addressing #66 : Consistently applying mask across error computation, cls error, and testing error/cls

Two further commits:

  • Allowing the config.cfg file to be used 'out of the box' by changing Piriform fields to ISBI2012
  • Requiring users to specify labels for datasets used during training. This was originally allowed for the full forward pass, but causes a vague error message for training sets, and the full forward pass no longer uses this section of the code.

@@ -39,7 +39,7 @@ train_conv_mode = fft

# cost function: square_loss, binomial_cross_entropy, softmax_loss, auto
# auto mode will match the out_type: boundary-softmax_loss, affinity-binomial_cross_entropy
cost_fn = auto
cost_fn = square_loss
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicholasturner1 why do you change it to square loss? I think that auto is better for default setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original thought was that it would be easier for new people to interpret/troubleshoot square error instead of having to take an extra step to figure out which error they're even using. Not a huge deal for me either way though. If you greatly prefer auto I'll swap it back

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I prefer auto. square loss corresponds to linear output. To help the new users, we should explain these in the comments in config.cfg file though.

@xiuliren
Copy link
Member

@nicholasturner1 could you solve the conflict? current code did not pass the autotest in travis.

@nicholasturner1
Copy link
Contributor Author

Nope, I hadn't changed things yet still (sorry...), but I also didn't merge it. @sachinravi14, do you know what the issues were?

@xiuliren
Copy link
Member

xiuliren commented Apr 4, 2016

@sachinravi14 did you make any fix of merging this pull request?

@sachinravi14
Copy link

No, I just merged and it worked for both synapse and semantic branches. Also, I think masking also needs to be fixed on test because I don't think the voxels are being counted correctly there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants