EGSnrc icon indicating copy to clipboard operation
EGSnrc copied to clipboard

Fix #894: revert egsphant to egs_brachy encoding

Open ftessier opened this issue 1 year ago • 10 comments

Revert to the egs_brachy encoding for .egsphant files, allowing for 63 media encoded with the set of alphanumeric characters (where 0 is reserved for vacuum).

Briefly, #633 expanded the number of media in .egsphant files to 95, using all printable ascii characters. However, the number of media was also independently expanded in egs_glib for the development of egs_brachy, but using a different encoding consisting of only the 63 alphanumeric ascii characters. This leads to inconsistent egsphant files that are no longer interchangeable.

This issue was originally reported and discussed in https://github.com/clrp-code/egs_brachy/issues/22.

ftessier avatar Jul 08 '22 23:07 ftessier

This is a quick solution, where the encoding is hard-coded in ctcreate and dosxyznrc. @blakewalters can you clean this up to put the variable definition inside a proper macro so that it is defined only once? Ideally we would add an encoding string inside the .egsphant file, but that can wait.

ftessier avatar Jul 08 '22 23:07 ftessier

All of the material type declarations for each voxel was on a new line in the egsphant, instead of printed in a slice-by-slice grid. My commit just fixed that.

rtownson avatar Jul 15 '22 12:07 rtownson

This is a quick solution, where the encoding is hard-coded in ctcreate and dosxyznrc. @blakewalters can you clean this up to put the variable definition inside a proper macro so that it is defined only once? Ideally we would add an encoding string inside the .egsphant file, but that can wait.

@blakewalters I added this macro, so you don't have to :)

rtownson avatar Jul 15 '22 12:07 rtownson

Can we try 6bb8618 instead, using advance='no' to prevent newlines? This is Fortran 90, is anyone still compiling with f77 you think?

ftessier avatar Jul 15 '22 12:07 ftessier

Can we try 6bb8618 instead, using advance='no' to prevent newlines? This is Fortran 90, is anyone still compiling with f77 you think?

Sure! I feel like we should be safe to switch to a standard that came out in 1991...

rtownson avatar Jul 15 '22 12:07 rtownson

Just leave the my other commit in the history, in case anyone ever comes to us wanting a version that works on f77.

rtownson avatar Jul 15 '22 12:07 rtownson

Just leave the my other commit in the history, in case anyone ever comes to us wanting a version that works on f77.

I will squash this before merging, but will leave a note in the commit message!

ftessier avatar Jul 15 '22 13:07 ftessier

In the end I reverted to using the implicit array over ii=1,iimax to avoid iimax write calls on each line. In my cursory testing, the implicit array form was significantly faster. For the record, the proper syntax of the write statement with advance='no' would be:

write(15, '(a1)', advance='no') encoding(code:code)

ftessier avatar Jul 19 '22 21:07 ftessier

Fixed spacing.

ftessier avatar Jul 19 '22 21:07 ftessier

Before merging: fix encoding string length typo in commit message. The encoding is 62 characters, and 0 is reserved for vacuum; so effectively 61 media can be encoded.

ftessier avatar Jul 19 '22 21:07 ftessier

Squashed the commits into a single one for merging into develop.

ftessier avatar Oct 31 '22 16:10 ftessier

There is still a discrepancy between this encoding and egs_brachy's: this one starts at 0, but egs_brachy's starts at 1. See here.

Or did I miss something in the discussion?

EDIT: Nevermind, I see it now at the top! My bad!

mchamberland avatar Dec 21 '22 21:12 mchamberland